-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adding new accounts summary query #2527
Conversation
WalkthroughThe updates enhance the leverage module by integrating account summaries, both individually and collectively. This involves adding new functions, parameters, and RPC methods to handle these summaries. The changes span across initialization logic, gRPC-web API, and CLI queries, ensuring comprehensive account data management. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/leverage/types/query.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
x/leverage/types/query.pb.gw.go
is excluded by!**/*.pb.gw.go
,!**/*.pb.gw.go
Files selected for processing (8)
- app/app.go (1 hunks)
- proto/umee/leverage/v1/query.proto (3 hunks)
- x/leverage/client/cli/query.go (2 hunks)
- x/leverage/keeper/accounts_summary.go (1 hunks)
- x/leverage/keeper/grpc_query.go (1 hunks)
- x/leverage/keeper/internal_test.go (2 hunks)
- x/leverage/keeper/keeper.go (4 hunks)
- x/leverage/keeper/suite_test.go (2 hunks)
Files not reviewed due to errors (2)
- x/leverage/keeper/keeper.go (no review received)
- proto/umee/leverage/v1/query.proto (no review received)
Files skipped from review due to trivial changes (1)
- x/leverage/keeper/grpc_query.go
Additional comments not posted (7)
x/leverage/keeper/internal_test.go (2)
7-7
: Import statement forauthkeeper
looks good.
26-37
: Changes toNewTestKeeper
function are consistent and correct.x/leverage/keeper/accounts_summary.go (3)
3-13
: Import statements are correctly added and necessary for the new functionality.
15-89
: TheaccountSummary
function is well-structured and handles errors appropriately.
92-121
: TheAccountsSummary
function correctly handles pagination and error cases.x/leverage/keeper/suite_test.go (1)
71-73
: Changes to theSetupTest
function are consistent and ensure correct test setup.app/app.go (1)
493-495
: Ensure the added parameters are correctly used and integrated.The addition of
keys[authtypes.StoreKey]
andapp.AccountKeeper
as parameters to theLeverageKeeper
initialization looks correct. Verify that these parameters are necessary and correctly used within theLeverageKeeper
implementation.Verification successful
The addition of
keys[authtypes.StoreKey]
andapp.AccountKeeper
as parameters to theLeverageKeeper
initialization appears to be correct. TheAccountKeeper
is used in both the implementation (keeper.go
) and test files (internal_test.go
). However,keys[authtypes.StoreKey]
does not appear to be directly referenced, which might indicate it is used indirectly or referenced differently.
authkeeper.AccountKeeper
is declared and used inkeeper.go
andinternal_test.go
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the added parameters in the LeverageKeeper implementation. # Test: Search for the usage of `keys[authtypes.StoreKey]` and `app.AccountKeeper` in the LeverageKeeper implementation. rg --type go 'keys\[authtypes.StoreKey\]' x/leverage/keeper/ rg --type go 'app.AccountKeeper' x/leverage/keeper/Length of output: 332
Script:
#!/bin/bash # Description: Verify the usage of the added parameters in the LeverageKeeper implementation. # Test: Search for the usage of `keys[authtypes.StoreKey]` in the LeverageKeeper implementation. rg --type go 'keys\[authtypes.StoreKey\]' x/leverage/keeper/ --files-without-match # Test: Search for the usage of `AccountKeeper` in the LeverageKeeper implementation. rg --type go 'AccountKeeper' x/leverage/keeper/ --files-without-matchLength of output: 3163
Script:
#!/bin/bash # Description: Extract specific lines where `keys[authtypes.StoreKey]` and `AccountKeeper` are used in the LeverageKeeper implementation. # Extract lines for `keys[authtypes.StoreKey]` rg --type go 'keys\[authtypes.StoreKey\]' x/leverage/keeper/ # Extract lines for `AccountKeeper` rg --type go 'AccountKeeper' x/leverage/keeper/Length of output: 602
Script:
#!/bin/bash # Description: Review specific lines in keeper.go and internal_test.go for correct usage of AccountKeeper. # Extract specific lines in keeper.go rg --type go 'authkeeper.AccountKeeper' x/leverage/keeper/keeper.go # Extract specific lines in internal_test.go rg --type go 'authkeeper.AccountKeeper' x/leverage/keeper/internal_test.goLength of output: 251
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/leverage/client/cli/query.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/leverage/client/cli/query.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/leverage/keeper/accounts_summary.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/leverage/keeper/accounts_summary.go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2527 +/- ##
===========================================
- Coverage 75.38% 62.60% -12.79%
===========================================
Files 100 276 +176
Lines 8025 15944 +7919
===========================================
+ Hits 6050 9982 +3932
- Misses 1589 5189 +3600
- Partials 386 773 +387
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Files not reviewed due to errors (1)
- CHANGELOG.md (no review received)
Additional Context Used
LanguageTool (12)
CHANGELOG.md (12)
Near line 115: Using ‘plenty’ without ‘of’ is considered to be informal.
Context: .../pull/2368) Fix inflow amount calculation. Previously, the inflow amount of the t...
Near line 217: Did you mean “limiting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...veragedLiquidate.MaxRepay` which allows to limit the liquidation size using the leverage...
Near line 350: The singular proper name ‘Bridge’ must be used with a third-person or a past tense verb.
Context: ...-network/umee/pull/1967) Gravity Bridge phase out phase-2: disable Umee -> Ethereum t...
Near line 351: The singular proper name ‘Bridge’ must be used with a third-person or a past tense verb.
Context: ...-network/umee/pull/1967) Gravity Bridge phase out phase-2: disable Umee -> Ethereum t...
Near line 401: Make sure that the singular noun after the number ‘4.1’ is correct.
Context: ...e/pull/1807) Fixes BNB ibc denom in 4.1 migration - [1812](https://github.com/umee-networ...
Near line 419: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...rowand
MsgRepay` won't return errors if there is nothing to withdraw, borrow or...
Near line 466: Possible missing article found.
Context: ...of the build process (you must build on same host as you run the binary, or copy the...
Near line 470: Possible typo: you repeated a word
Context: ...e/pull/1555) Updates IBC to v5.1.0 that adds adds optional memo field to `FungibleTokenPa...
Near line 527: A comma may be missing after the conjunctive/linking adverb ‘Also’.
Context: ...ableLend, docs, and internal functions. Also QueryLoaned similar queries to QuerySup...
Near line 531: This word is normally spelled as one.
Context: ...roto getters in x/leverage and x/oracle proto types. - [1126](https://github.com/umee-netwo...
Near line 552: Possible missing comma found.
Context: ...umee/pull/1157) AddedPrintOrErr
util function optimizing the CLI code flow. - [1118](...
Near line 650: Possible typo: you repeated a word
Context: ...k/umee/pull/1358/files) Disable Gravity Bridge bridge messages. ### Improvements - [#1355](...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- it's enough to iterate accounts that supplies something. Account that didn't supply anything can't collaterize nor borrow
- There are typos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/leverage/types/query.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
x/leverage/types/query.pb.gw.go
is excluded by!**/*.pb.gw.go
,!**/*.pb.gw.go
Files selected for processing (3)
- proto/umee/leverage/v1/query.proto (3 hunks)
- x/leverage/client/cli/query.go (2 hunks)
- x/leverage/keeper/accounts_summary.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- x/leverage/client/cli/query.go
- x/leverage/keeper/accounts_summary.go
Additional comments not posted (4)
proto/umee/leverage/v1/query.proto (4)
9-9
: Import statement for pagination added.
58-62
: New RPC methodAccountSummaries
added to theQuery
service.
351-355
: New messageQueryAccountSummaries
added with pagination support.
357-368
: New messagesAccountSummary
andQueryAccountSummariesResponse
added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/leverage/keeper/accounts_summary.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/leverage/keeper/accounts_summary.go
Description
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...